feat: update cooldown handling to support async operations #2823
Conversation
Lumabots
changed the title
… + correct typehint of max_concurrency since only ctx is passed and never message
|
@Soheab brought to my attention that this PR might be breaking — specifically, the renaming of the message parameter to ctx could cause issues if someone is using a message cooldown, like with get_bucket(message=message). If they’re not using get_bucket(message=...), then there shouldn’t be any problems, since both message and ctx share all the internal attributes being accessed. That said, I’m personally against keeping the parameter named message, especially since slash commands don’t even have a message attribute. Let me know how you’d like to handle it. I don’t think this should be an issue — using ctx instead of message shouldn’t break anything unless a user is specifically calling that function, which I doubt many people do. |
Signed-off-by: Lumouille <144063653+Lumabots@users.noreply.github.com>
Lulalaby
force-pushed
the
master
branch
2 times, most recently
from
b55c125 to
82659b2
Compare
Lulalaby
requested review from
Paillat-dev
and removed request for
Dorukyum and
ChickenDevs
discord/ext/commands/cooldowns.py
Outdated
| from ...ext.commands import Context | ||
|
|
||
| if isinstance(ctx, Context): | ||
| result = self._factory(ctx.message) | ||
| else: | ||
| result = self._factory(ctx) | ||
| if inspect.isawaitable(result): | ||
| return await result | ||
| return result |
There was a problem hiding this comment.
This all can be simplified with:
| from ...ext.commands import Context | |
| if isinstance(ctx, Context): | |
| result = self._factory(ctx.message) | |
| else: | |
| result = self._factory(ctx) | |
| if inspect.isawaitable(result): | |
| return await result | |
| return result | |
| return await utils.maybe_coroutine(self._factory, ctx) |
Requires importing from discord import utils
Co-authored-by: DA344 <108473820+DA-344@users.noreply.github.com> Signed-off-by: Lala Sabathil <aiko@aitsys.dev>
|
well it was not really ready for review since i thought it was going to be postpone for next, i should have kept it as draft |
|
yeah as i thought, this cannot be merge for V2.7 as it will be breaking for function like is_on_cooldown |
Co-authored-by: Soheab <33902984+Soheab@users.noreply.github.com> Signed-off-by: Lumouille <144063653+Lumabots@users.noreply.github.com>
Summary
it add the possibility to use async function for cooldown, it also change the internal function to use ctx instead of only message since slash commands dont have a message
Nothing should be breaking
Information
examples, ...).
Checklist
type: ignorecomments were used, a comment is also left explaining why.